-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
optimize array tracking (fix #4318) #9511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Size ReportBundles
Usages
|
Thanks for following up! You can go against the |
|
@johnsoncodehk Thanks, I based the PR on |
|
Hope this can be landed into the 3.4. |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
Great job! I've merged it with the latest reactivity system refactors on minor and everything is working. Ecosystem CI failures are expected and match current fails on minor. Regarding the open points:
|
|
👍 awesome |
… array identity methods (vuejs#11328) related: vuejs#9511
This PR implements the optimisations proposed in #4318. Shortly:
ARRAY_ITERATE_KEY, which represents a full dependency on an array (not including extra keys when handling an array as an object).lengthor an integer key is triggered on an array.ARRAY_ITERATE_KEY.Unrelated changes included in this PR
arrayInstrumentationsobject use anullprototype -> no need to check forownKeyswhen accessing, should be ever so slightly faster on misses.indexOfand co. I optimized the existing instrumentation slightly: when the needle is not found and is not a proxy, there's no reason to do a 2nd pass.New public APIs
@vue/reactivityexposesreadArray(source, deep = false).This is an advanced reactivity function that returns the underlying raw array and tracks it entirely.
As this is a performance oriented API, an extra parameter indicates if deep reactive arrays return proxified array items (false by default). Of course,
deep: truecomes at the cost of one array copy (vs zero).When
sourceis not reactive, it's returned as-is and no tracking occurs.Why expose this? Because it's still useful for performance-conscious users when they want to perform an operation not well covered by built-in array functions. Examples:
Open points
indexOfand co. This seems useful on readonly arrays too?v-forvery close to its current source code, sticking to thefor (i = 0..)loop. UsingreadArray(source, true)implies a full array copy on deeply reactive arrays. I think it'd be better to usemapor an iterator (also instrumented for perf, requires no O(N) allocation) or maybereadArray(source, false)and calltoReactiveinside the loop if required (more code).Object.groupByAPI above. It's unfortunate it's not an array instance method. Do we want to patchObject? It feels really bad to patch native objects, but this is an API I can imagine being used insidecomputeda lot in the future... so it'd be nice to have the best perf by default. Maybe let users opt into that?Noteworthy implementation details
iterator()about the timing of tracking that is worth reading.find,someorevery. After this PR they will track the full array and may trigger when not strictly necessary.This is not totally new, the existing instrumentations of
indexOfand co. did exactly this already.Might be "fixed" if we introduce range dependencies (see below).
Further ideas
If this PR is merged, some ideas:
indexOf,lastIndexOf,find,findLast,findIndex,findLastIndex,some,every,includes,slice(not instrumented in this PR).